Skip to content

Simplify AsyncView - #11

Merged
devgioele merged 1 commit into
mainfrom
async-view-inference
Feb 16, 2023
Merged

Simplify AsyncView#11
devgioele merged 1 commit into
mainfrom
async-view-inference

Conversation

@devgioele

@devgioele devgioele commented Feb 15, 2023

Copy link
Copy Markdown

This PR defines the component AsyncView without using AsyncState. This simplifies the function and avoids type casting.

AsyncState is still made available, but marked as deprecated to discourage its use. A future major release may completely remove the AsyncState enum.

@devgioele devgioele added the enhancement New feature or request label Feb 15, 2023
@devgioele
devgioele requested a review from lukasvice February 15, 2023 15:11
@devgioele devgioele self-assigned this Feb 15, 2023
@lukasvice

Copy link
Copy Markdown
Member

Could you please provide an example of how it disables type inference?

@devgioele

devgioele commented Feb 16, 2023

Copy link
Copy Markdown
Author

Using AsyncState disables type inference, because by first calling getAsyncState and then acting based on its result, Typescript can no longer infer whether data or error are available. Thus it requires type casting with the keyword as or optional chaining with ?..

If one was to use AsyncState, he could write something like the following:

const asyncState = getAsyncState(data, error);

return (
  <div>
      {asyncState === AsyncState.FINISHED_WITH_ERROR && (
        <RequestError>{error?.response?.data.message}</RequestError>
      )}
  </div>
)

This requires the use of error? instead of simply error, because Typescript cannot understand that if getAsyncState returns FINISHED_WITH_ERROR, error must be defined. In this sense the use of getAsyncState disables type inference. It can and should be avoided.

@lukasvice

Copy link
Copy Markdown
Member

Thanks for the explanation. Let's get @mmalfertheiner on this, since I don't know all the implications of this change.

@mmalfertheiner mmalfertheiner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@devgioele
devgioele merged commit 3d2946b into main Feb 16, 2023
@devgioele
devgioele deleted the async-view-inference branch February 16, 2023 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants